Did you run the tests after making this change?

@kristoferjoseph Looks as if I omitted the tests, unfortunately. Tried now and one test is failing.

@kristoferjoseph Specifically, the test 'input values get copied' fails. Why is it that the value of the mutated element should be kept, as opposed to be updated to the value of the mutating element?

@kristoferjoseph The test in question doesn't really make sense to me, I think we better resolve how this is supposed to work before we go any further. To me, it seems that the test should assert that newEl.value gets copied to el.value. Is this wrong?

@kristoferjoseph It also ensures that new values don't get copied over however, I just checked.

What if you want to erase an input's value when you re-render, how do you control this if yo-yo ignores this change? Doesn't seem to make sense to me.

The pattern that would make sense to me is to record when an input's value changes and render this into DOM updates before passing them to yo-yo.

@kristoferjoseph I see now that yo-yo checks if the mutating element has value different from null. However, this actually doesn't detect that the test sets newEl.value directly, so maybe there's a bit of confusion due to this.

@kristoferjoseph I rewrote the original test slightly and added a new one, asserting that new input values get copied if they are actually set.

However, I have to deal with the select case.

@kristoferjoseph I tested one more thing, the case where you clear the value via the mutating element. I tested by assigning an empty string to value, and I can see that the old value is not overwritten. Does this make sense to you? To me it seems wrong. I would expect to be able to clear the input value via mutation.

Nevermind: This actually works, not sure what I did wrong before.

I've reverted to the original behaviour and added a test for the clearing of input value case.

Will have to add handling of selects, as per the bug I experienced.

@kristoferjoseph Does it ever make sense to keep the selectedIndex (which yo-yo attempted to do via value) on a select? Can it ever be null?

Even if the select has no elements, the selectedIndex will be -1 and in that case I don't see any point in maintaining it as the mutated select will lose all its children anyway.

I've made the change in my branch now to not change the selection state of select elements. I don't see any case where it makes sense, although I could be missing something.

@aknuds1 as for keeping the selected index. Imagine a scenario where you have set the select index then update something else in the same scope. You want to make sure the index is maintained during an unrelated update.

@kristoferjoseph How would you formulate this into a test case? I can't see any way that selectedIndex would be null, so there's no way to tell whether it's supposed to be updated or not. That's why it becomes so tricky to try to tell whether the incoming mutation is trying to change a value or not.

I think it's way safer to set all properties you need on the mutating element, rather than to expect yo-yo to maintain them.

@aknuds1 https://github.com/yoshuawuyts/nanomorph/pull/28/files

@kristoferjoseph That doesn't show how to test this for select elements though, which is what I would need to know? Like I said, I cannot see any way to do it in practice.

Ah, sorry, was actually replying to one of your earlier questions. I will make a select test as well. Was hoping this patch might fix some of the other issues you were seeing.

@kristoferjoseph Is it necessarily a great idea to implement this sort of intelligence in nanomorph? I suspect it violates the principle of least surprise, at least I was surprised to find that yo-yo tries to preserve old values as opposed to copying directly from the incoming/mutating element.

Does morphdom function in this way too?

@kristoferjoseph Yeah it makes it more clear, but I'm just not so sure a morphing library should do this. It sounds like a decision to be made on top of the morphing, i.e. it might be undesirable to the user of the morphing library. The value preservation algorithm certainly is imperfect as it stands, as my scenario with the select elements shows.

I'm discussing with @yoshuawuyts however, and he explained why it's useful in the Choo context. I think it really depends on higher level concerns (such as Choo), so I'm just not so sure the behaviour should be hardcoded into nanomorph. I don't know how it works in morphdom, but I suspect it doesn't have this sort of intelligence built in, since yo-yo is implementing it on top of it. I suspect it would be better handled by a module around nanomorph.

I'm discussing with @yoshuawuyts however

@kristoferjoseph come to IRC 🕥 ✨ 🎉

It's good we're having this discussion though, I wasn't aware of the issues you can run into without preserving values in the Choo context until @yoshuawuyts explained it to me.

@kristoferjoseph http://irccloud.com/ and then join freenode and #choo :D - I believe in you